Add function to append item to list#1
Conversation
WalkthroughA new function Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@random.py`:
- Around line 1-3: The function add_item uses a mutable default argument
items=[], causing shared state across calls; change the signature to default
items to None and inside add_item check if items is None then assign items = []
before appending, ensuring each call without an explicit items receives a fresh
list (update the parameter and the initialization logic in add_item).
- Line 1: The file name random.py shadows Python's stdlib random module; rename
the file (e.g., items_helper.py or collection_utils.py) and update any imports
that reference this module to the new name, then run tests/grep to ensure
nothing else imports the old random.py; also keep the function add_item(item,
items=[]) but change its default mutable argument to avoid shared-state bugs
(e.g., accept items=None and set items=[] inside add_item) and update any call
sites accordingly.
| def add_item(item, items=[]): | ||
| items.append(item) | ||
| return items |
There was a problem hiding this comment.
Mutable default argument causes shared state across calls.
Using items=[] as a default creates a single list object at function definition time that is reused across all calls. Successive calls without an explicit items argument will accumulate results:
add_item("a") # returns ["a"]
add_item("b") # returns ["a", "b"] — not ["b"]This is flagged by Ruff B006. The standard fix is to default to None and create a new list inside the function body.
🐛 Proposed fix
-def add_item(item, items=[]):
+def add_item(item, items=None):
+ if items is None:
+ items = []
items.append(item)
return items🧰 Tools
🪛 Ruff (0.15.2)
[warning] 1-1: Do not use mutable data structures for argument defaults
Replace with None; initialize within function
(B006)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@random.py` around lines 1 - 3, The function add_item uses a mutable default
argument items=[], causing shared state across calls; change the signature to
default items to None and inside add_item check if items is None then assign
items = [] before appending, ensuring each call without an explicit items
receives a fresh list (update the parameter and the initialization logic in
add_item).
| @@ -0,0 +1,3 @@ | |||
| def add_item(item, items=[]): | |||
There was a problem hiding this comment.
Filename random.py shadows Python's standard library module.
Naming this file random.py will shadow the built-in random module. Any import random elsewhere in the project (or its dependencies) would import this file instead. Consider renaming to something more descriptive.
🧰 Tools
🪛 Ruff (0.15.2)
[warning] 1-1: Do not use mutable data structures for argument defaults
Replace with None; initialize within function
(B006)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@random.py` at line 1, The file name random.py shadows Python's stdlib random
module; rename the file (e.g., items_helper.py or collection_utils.py) and
update any imports that reference this module to the new name, then run
tests/grep to ensure nothing else imports the old random.py; also keep the
function add_item(item, items=[]) but change its default mutable argument to
avoid shared-state bugs (e.g., accept items=None and set items=[] inside
add_item) and update any call sites accordingly.
1 participant
Summary by CodeRabbit